OCPBUGS-32776: Fix IBM Public Cloud DNS Provider Update Logic #1133

gcs278 · 2024-08-22T22:46:16Z

The IBM Public Cloud DNS provider (cis_provider.go) had a bug in createOrUpdateDNSRecord where it checked for the existence of a DNS record by filtering both DNS name and target. If the target was updated (e.g., due to a load balancer recreation), the logic would not match the existing DNS record. As a result, the function would attempt to create a new record, but fail because a record with that name already existed, as multiple DNS records with the same name are not allowed.

The fix is to remove the filtering by target and rely solely on filtering by name, as the name is the only attribute that needs to be unique.

Additionally, the IBM DNS logic doesn't work for multiple targets and this creates unexpected and problematic results. The logic has been refactored to only create and delete using the first target. It warns the user when multiple targets are set.

This PR also includes some unit test fix up and missing unit test coverage for the IBM CIS Provider.

This resolves the same DNS issues for public PowerVS cloud as it uses the same logic.

openshift-ci-robot · 2024-08-22T22:46:21Z

@gcs278: This pull request references Jira Issue OCPBUGS-32776, which is invalid:

expected the bug to target the "4.18.0" version, but no target version was set

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

The bug has been updated to refer to the pull request using the external bug tracker.

In response to this:

The IBM Public Cloud DNS provider (cis_provider.go) had a bug in createOrUpdateDNSRecord where it checked for the existence of a DNS record by filtering both DNS name and target. If the target was updated (e.g., due to a load balancer recreation), the logic would not match the existing DNS record. As a result, the function would attempt to create a new record, but fail because a record with that name already existed, as multiple DNS records with the same name are not allowed.

The fix is to remove the filtering by target and rely solely on filtering by name, as the name is the only attribute that needs to be unique.

This PR also includes some unit test fix up and missing coverage for the IBM CIS Provider.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

gcs278 · 2024-08-22T22:57:45Z

/jira refresh

openshift-ci-robot · 2024-08-22T22:57:51Z

@gcs278: This pull request references Jira Issue OCPBUGS-32776, which is valid. The bug has been moved to the POST state.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target version (4.18.0) matches configured target version for branch (4.18.0)
bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @lihongan

In response to this:

/jira refresh

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

openshift-ci-robot · 2024-08-22T22:58:26Z

@gcs278: This pull request references Jira Issue OCPBUGS-32776, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target version (4.18.0) matches configured target version for branch (4.18.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @lihongan

In response to this:

The IBM Public Cloud DNS provider (cis_provider.go) had a bug in createOrUpdateDNSRecord where it checked for the existence of a DNS record by filtering both DNS name and target. If the target was updated (e.g., due to a load balancer recreation), the logic would not match the existing DNS record. As a result, the function would attempt to create a new record, but fail because a record with that name already existed, as multiple DNS records with the same name are not allowed.

The fix is to remove the filtering by target and rely solely on filtering by name, as the name is the only attribute that needs to be unique.

This PR also includes some unit test fix up and missing coverage for the IBM CIS Provider.

This also resolves the same DNS issues for public PowerVS cloud as it uses the same logic.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

gcs278 · 2024-08-26T16:10:14Z

I missed one detail of the bug, missing instructions in the Progressing status condition for PowerVS when you change scope. Looking into it.
/hold

gcs278 · 2024-08-26T19:30:02Z

@lihongan Added the missing scope change instructions for the PowerVS type.

/unhold

gcs278 · 2024-08-27T00:21:55Z

infra failures
/retest

gcs278 · 2024-08-28T00:28:35Z

Infra issues
/retest

lihongan · 2024-08-28T05:32:39Z

pre-merge tested on OpenStack and looks good now

$ oc -n openshift-ingress-operator patch ingresscontroller/intlb --type=merge --patch='{"spec":{"endpointPublishingStrategy": {"type":"LoadBalancerService", "loadBalancer": {"scope":"External"}}}}'
ingresscontroller.operator.openshift.io/intlb patched

$ oc get co/ingress
NAME      VERSION                                                   AVAILABLE   PROGRESSING   DEGRADED   SINCE   MESSAGE
ingress   4.17.0-0.ci.test-2024-08-28-040831-ci-ln-13rxbgt-latest   True        True          False      23m     ingresscontroller "intlb" is progressing: IngressControllerProgressing: One or more status conditions indicate progressing: LoadBalancerProgressing=True (OperandsProgressing: One or more managed resources are progressing: The IngressController scope was changed from "Internal" to "External".  To effectuate this change, you must delete the service: `oc -n openshift-ingress delete svc/router-intlb`; the service load-balancer will then be deprovisioned and a new one created.  This will most likely cause the new load-balancer to have a different host name and IP address from the old one's.  Alternatively, you can revert the change to the IngressController: `oc -n openshift-ingress-operator patch ingresscontrollers/intlb --type=merge --patch='{"spec":{"endpointPublishingStrategy":{"loadBalancer":{"scope":"Internal"}}}}'`).

$ oc -n openshift-ingress delete svc/router-intlb
service "router-intlb" deleted

$ oc -n openshift-ingress get svc/router-intlb
NAME           TYPE           CLUSTER-IP      EXTERNAL-IP    PORT(S)                      AGE
router-intlb   LoadBalancer   172.30.110.89   38.102.83.53   80:32114/TCP,443:30834/TCP   4m23s

will test on IBMCloud later

lihongan · 2024-08-28T09:53:50Z

pre-merge tested on IBMCloud and also looks good

$ oc -n openshift-ingress get svc/router-intlb
NAME           TYPE           CLUSTER-IP      EXTERNAL-IP                         PORT(S)                      AGE
router-intlb   LoadBalancer   172.30.111.21   53084b0d-eu-de.lb.appdomain.cloud   80:31026/TCP,443:32398/TCP   78s

$ oc -n openshift-ingress-operator patch ingresscontroller/intlb --type=merge --patch='{"spec":{"endpointPublishingStrategy": {"type":"LoadBalancerService", "loadBalancer": {"scope":"External"}}}}'
ingresscontroller.operator.openshift.io/intlb patched

$ oc -n openshift-ingress delete svc/router-intlb
service "router-intlb" deleted

$ oc -n openshift-ingress get svc/router-intlb
NAME           TYPE           CLUSTER-IP      EXTERNAL-IP                         PORT(S)                      AGE
router-intlb   LoadBalancer   172.30.242.68   72736508-eu-de.lb.appdomain.cloud   80:32041/TCP,443:30579/TCP   65s

$ oc -n openshift-ingress-operator get ingresscontroller/intlb -oyaml
<......>
  - lastTransitionTime: "2024-08-28T09:24:02Z"
    message: The record is provisioned in all reported zones.
    reason: NoFailedZones
    status: "True"
    type: DNSReady

pkg/operator/controller/ingress/load_balancer_service.go

Miciah · 2024-08-28T15:17:17Z

/assign

Miciah · 2024-08-28T15:17:54Z

@SzucsAti, this is a follow-up to #796. Are you available to review the changes to the IBM DNS provider?

SzucsAti

Looks good to me, thank you for the fix!

gcs278 · 2024-09-16T21:06:14Z

infra failures
/retest

gcs278 · 2024-09-16T21:26:26Z

Removed WIP, sorry for the delay, I am open to reviews again @candita @Miciah @SzucsAti

Updates since last review:

I added more unit test coverage.
- I felt it necessary to fix existing bugs in the unit tests and add more coverage as it provides confidence that I didn't perturb the DNS provider logic in an unintentional way.
- The unit tests being in separate commits is intentional and provides a valuable diff before & after the actual bug fix here. You can review the last commit and see there were a few required unit test updates which are results of the bug fixes I added.
I removed create/update support for multiple targets in DNSRecords for IBM Cloud.
- First, the code was misleading because IBM DNS doesn't support it, and therefore it doesn't work (it hot loops and overwrites records in IBM private cloud).
- Second, since the bug fix actually impacts the way that multi-targeted DNSRecords would get created in IBM Public Cloud, it seemed logical to fix this right now, otherwise, we'd create a future headache for maintaining compatibility (if we were ever concerned about compatibility here)

gcs278 · 2024-09-16T21:28:48Z

Keeping the hold while I wait to see what the results of adding a IBM Cloud job are openshift/release#56785

/hold

gcs278 · 2024-09-17T14:25:28Z

/retest

gcs278 · 2024-10-28T16:11:20Z

Now that openshift/release#56785 has merged, though it will most likely fail due to DNS Propagation issues (being tested in #1132), I'm curious to see if we see anything interesting:
/test e2e-ibmcloud-operator

gcs278 · 2024-10-30T18:05:34Z

I don't think it's enabled, but just curious:
/testwith e2e-ibmcloud-operator #1164

gcs278 · 2024-10-30T19:02:28Z

/testwith openshift/cluster-ingress-operator/master/e2e-ibmcloud-operator #1164

gcs278 · 2024-11-11T22:36:15Z

After introducing and running e2e-ibmcloud-operator on different PRs, I realized it was passing successfully without this fix. I was mistaken that our E2E would have caught this bug. However, it looks like adding a Availabile=True condition check to TestScopeChange after deleting the service ensures test coverage for bugs like this. Updated this PR to add that.

Now let's test to make sure everything works:
/testwith openshift/cluster-ingress-operator/master/e2e-ibmcloud-operator #1164

gcs278 · 2024-11-12T01:33:44Z

Forgot to push...
/testwith openshift/cluster-ingress-operator/master/e2e-ibmcloud-operator #1164

gcs278 · 2024-11-12T01:38:45Z

/testwith openshift/cluster-ingress-operator/master/e2e-ibmcloud-operator openshift/cluster-ingress-operator/pull#1164

openshift-ci · 2024-11-12T01:38:49Z

@gcs278, testwith: Error processing request. ERROR:

could not determine job runs: invalid format for additional PR: openshift/cluster-ingress-operator/pull#1164

gcs278 · 2024-11-12T01:39:09Z

/testwith openshift/cluster-ingress-operator/master/e2e-ibmcloud-operator https://github.com/openshift/cluster-ingress-operator#1164

gcs278 · 2024-11-12T14:25:50Z

multi-pr-openshift-cluster-ingress-operator-1133-openshift-cluster-ingress-operator-1164 passed which has E2E test coverage for this bug, proving it resolved the problem.

gcs278 · 2024-11-12T18:11:31Z

/retest-required

gcs278 · 2024-12-05T15:47:12Z

/retest

pkg/dns/ibm/private/dnssvcs_provider.go

candita · 2024-12-05T19:34:01Z

pkg/dns/ibm/private/dnssvcs_provider.go

-			case iov1.CNAMERecordType:
-				inputRData, err := p.dnsService.NewResourceRecordInputRdataRdataCnameRecord(target)
+
+			// TODO DNS record update should handle the case where we have an A record and want a CNAME record or vice versa


I see this is a carried-over TODO, but should it be done now in order to fix the bug? Or is it out of scope?

I think it's out of scope for this bug since it can be fixed independently.

This TODO is for when someone changes the DNSRecord type from CNAME to A, or visa versa, that it won't update the dns record type. That's not a big deal for OpenShift actually, because IBMCloud only creates CNAME records, but I can see it as a low priority "completeness" TODO.

pkg/dns/ibm/public/cis_provider.go

candita · 2024-12-05T19:45:50Z

pkg/dns/ibm/public/cis_provider.go

+		}
+	}
+	if result == nil || result.Result == nil {
+		return fmt.Errorf("createOrUpdateDNSRecord: ListAllDnsRecords returned nil as result")


What happens for the very first record created? We can't find any to update, but we need to go on to create it instead of returning an error, don't we?

Hmm, that's a good question. I found that result and result.Result are both initialized when creating the DNS record for the first time. The results gets filtered for the record.Spec.DNSName, so whether it's the very first DNS record in the hosted zone isn't a factor.

I agree it's a bit odd to check for result.Result, but the original author wrote this logic, I haven't update it as works as intended so far, and it's possible there is a subtle reason why it's here.

candita · 2024-12-05T23:53:10Z

pkg/dns/ibm/public/cis_provider_test.go

+			createDnsRecordInputOutput: dnsclient.CreateDnsRecordInputOutput{
+				InputId:        "testCreate",
+				OutputError:    nil,
+				OutputResponse: &core.DetailedResponse{StatusCode: http.StatusOK},


Is it possible to find a way to check that the list was updated?

Same comment as above, there is no state to check.

candita · 2024-12-05T23:54:26Z

pkg/dns/ibm/public/cis_provider_test.go

+				OutputError:    errors.New("error in CreateDnsRecord"),
+				OutputResponse: &core.DetailedResponse{StatusCode: http.StatusRequestTimeout},
+			},
+			expectErrorContains: "error in CreateDnsRecord",


This doesn't really seem to be testing any error input if we're just matching the supplied error.

Hmm, I view OutputError as mocking a "fake" Upstream DNS Server as if it was providing an error. This is confirming that we are getting the error back that we told the "fake" Upstream DNS Server to generate, just like what would happen if there was a real DNS Server error.

We have other test cases that test error messages produced by our own provider code, but this error checking is still valuable because it proves the functions are returning the error we provided when the fake response is StatusRequestTimeout.

candita · 2024-12-06T00:01:54Z

pkg/dns/ibm/private/client/fake_client.go

 func (fdc FakeDnsClient) ListResourceRecords(listResourceRecordsOptions *dnssvcsv1.ListResourceRecordsOptions) (result *dnssvcsv1.ListResourceRecords, response *core.DetailedResponse, err error) {
-	fakeListDnsrecordsResp := &dnssvcsv1.ListResourceRecords{}
-	recordType := string(iov1.ARecordType)
-	rData := map[string]interface{}{"ip": fdc.ListAllDnsRecordsInputOutput.RecordTarget}
-
-	fakeListDnsrecordsResp.ResourceRecords = append(fakeListDnsrecordsResp.ResourceRecords, dnssvcsv1.ResourceRecord{ID: &fdc.ListAllDnsRecordsInputOutput.RecordName, Name: &fdc.ListAllDnsRecordsInputOutput.RecordName, Type: &recordType, Rdata: rData})
-
-	resp := &core.DetailedResponse{
-		StatusCode: fdc.ListAllDnsRecordsInputOutput.OutputStatusCode,
-		Headers:    map[string][]string{},
-		Result:     result,
-		RawResult:  []byte{},
-	}
-
-	return fakeListDnsrecordsResp, resp, fdc.ListAllDnsRecordsInputOutput.OutputError
+	return fdc.ListAllDnsRecordsInputOutput.OutputResult, fdc.ListAllDnsRecordsInputOutput.OutputResponse, fdc.ListAllDnsRecordsInputOutput.OutputError


listResourceRecordsOptions is unused. Also, can you add a comment about what is actually being returned here? We haven't changed anything.

Correct, listResourceRecordsOptions was always unused. The options would only really be helpful if we were able to test the IBM Cloud DNS Provider, as it's an input struct for their API calls.

We build a ListAllDnsRecordsInputOutput object as an input to each of the unit test cases, and it has the OutputResult, OutputResponse, and OutputError. In this function, we just return them so we can continue testing our functions (delete and createOrUpdateDNSRecord)

I'll add a comment.

candita · 2024-12-06T00:07:16Z

pkg/dns/ibm/private/dnssvcs_provider_test.go

+	"k8s.io/utils/pointer"
+
+	"github.com/IBM/go-sdk-core/v5/core"
+	"github.com/IBM/networking-go-sdk/dnssvcsv1"


Same comment for imports not in order.

candita · 2024-12-06T00:08:37Z

pkg/dns/ibm/private/dnssvcs_provider_test.go

I'll review this further after learning more about my questions in cis_provider_test.go

Both IBM public and private cloud unit tests were missing unit test coverage. This update extends test coverage for the Delete and CreateOrUpdateRecord functions. This commit provides an important point of reference for future commits that may preturb the existing functionality. Both Test_createOrUpdateDNSRecord functions previously only tested the update logic. In order to test the create logic, `CreateDNSRecord` and `CreateResourceRecord` needed to be implemented in the public and private `fake_client.go` respectively. The new test cases required the ability to control the response and results of `ListAllDnsRecords`, which were previously hardcoded in both public and private IBM cloud unit tests. Both public and private unit tests were updated to use the new OutputResults field specified in the `ListAllDnsRecordsInputOutput` struct, allowing the new test cases to specify no result (indicating no existing DNS record) so we can trigger the create logic. Lastly, various test cases were added to cover untested scenarios, such as testing CNAME Records, mismatching targets, missing record types or IDs, handling nil results, etc.

The IBM Public Cloud DNS provider (`cis_provider.go`) had a bug in `createOrUpdateDNSRecord` where it checked for the existence of a DNS record by filtering both DNS name and target. If the target was updated (e.g., due to a load balancer recreation), the logic would not match the existing DNS record. As a result, the function would attempt to create a new record, but fail because a record with that name already existed, as multiple DNS records with the same name are not allowed in IBM Cloud DNS providers. The fix is to remove the filtering by target and rely solely on filtering by name, as the name is the only attribute that needs to be unique. Additionally, the IBM DNS logic doesn't work for multiple targets and this creates unexpected and problematic results. The logic has been refactored to only create using the first target and it warns the user when multiple targets are set. This change is low risk since the Ingress Operator will never create a DNSRecord with multiple targets in `desiredDNSRecord`. Modify TestScopeChange to check for `Available=True` after deleting the service to add test coverage for this issue.

openshift-ci · 2024-12-23T17:13:46Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from candita. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

gcs278

@candita thanks for the detailed review. Should be ready for your review again.

gcs278 · 2024-12-11T00:57:38Z

pkg/dns/ibm/public/client/fake_client.go

It looks like they are implementing two different DNSClient interfaces (public and private).

gcs278 · 2024-12-11T01:09:04Z

pkg/dns/ibm/private/client/fake_client.go

 func (fdc FakeDnsClient) ListResourceRecords(listResourceRecordsOptions *dnssvcsv1.ListResourceRecordsOptions) (result *dnssvcsv1.ListResourceRecords, response *core.DetailedResponse, err error) {
-	fakeListDnsrecordsResp := &dnssvcsv1.ListResourceRecords{}
-	recordType := string(iov1.ARecordType)
-	rData := map[string]interface{}{"ip": fdc.ListAllDnsRecordsInputOutput.RecordTarget}
-
-	fakeListDnsrecordsResp.ResourceRecords = append(fakeListDnsrecordsResp.ResourceRecords, dnssvcsv1.ResourceRecord{ID: &fdc.ListAllDnsRecordsInputOutput.RecordName, Name: &fdc.ListAllDnsRecordsInputOutput.RecordName, Type: &recordType, Rdata: rData})
-
-	resp := &core.DetailedResponse{
-		StatusCode: fdc.ListAllDnsRecordsInputOutput.OutputStatusCode,
-		Headers:    map[string][]string{},
-		Result:     result,
-		RawResult:  []byte{},
-	}
-
-	return fakeListDnsrecordsResp, resp, fdc.ListAllDnsRecordsInputOutput.OutputError
+	return fdc.ListAllDnsRecordsInputOutput.OutputResult, fdc.ListAllDnsRecordsInputOutput.OutputResponse, fdc.ListAllDnsRecordsInputOutput.OutputError


Correct, listResourceRecordsOptions was always unused. The options would only really be helpful if we were able to test the IBM Cloud DNS Provider, as it's an input struct for their API calls.

We build a ListAllDnsRecordsInputOutput object as an input to each of the unit test cases, and it has the OutputResult, OutputResponse, and OutputError. In this function, we just return them so we can continue testing our functions (delete and createOrUpdateDNSRecord)

I'll add a comment.

gcs278 · 2024-12-11T01:38:35Z

pkg/dns/ibm/private/client/fake_client.go

-func (FakeDnsClient) CreateResourceRecord(createResourceRecordOptions *dnssvcsv1.CreateResourceRecordOptions) (result *dnssvcsv1.ResourceRecord, response *core.DetailedResponse, err error) {
-	return nil, nil, nil
+func (fdc FakeDnsClient) CreateResourceRecord(createResourceRecordOptions *dnssvcsv1.CreateResourceRecordOptions) (result *dnssvcsv1.ResourceRecord, response *core.DetailedResponse, err error) {
+	if fdc.CreateDnsRecordInputOutput.InputId != *createResourceRecordOptions.Name {


I did this mainly to be consistent with the existing patterns, but it ensures that the
incoming createResourceRecordOptions.Name parameter is getting configured correctly. InputID is equivalent to the name, it's just named generically to keep all update/create/delete structs similar.

I'll add a comment.

gcs278 · 2024-12-11T02:08:53Z

pkg/dns/ibm/public/cis_provider_test.go

 	dnsclient "github.com/openshift/cluster-ingress-operator/pkg/dns/ibm/public/client"
+
+	"k8s.io/utils/pointer"


I'll do that, but what about https://github.com/go-imports-organizer/goio/blob/main/examples/openshift_kubernetes.yaml? Do we have an official style guide of how we are supposed to order go imports?

pkg/dns/ibm/public/cis_provider_test.go

gcs278 · 2024-12-23T16:27:55Z

pkg/dns/ibm/private/dnssvcs_provider.go

+	for _, resourceRecord := range listResult.ResourceRecords {
+		if *resourceRecord.Name == dnsName {
+			if resourceRecord.ID == nil {
+				return fmt.Errorf("createOrUpdateDNSRecord: record id is nil")


This was added because if the resourceRecord.ID is nil, you'll get a nil pointer dereference later when going to delete it, so no value in continuing. I added it because I saw cis_provider.go had it, but this provider didn't have it:

cluster-ingress-operator/pkg/dns/ibm/public/cis_provider.go

Line 124 in dbdff16

return fmt.Errorf("delete: record id is nil")

gcs278 · 2024-12-23T16:33:28Z

pkg/dns/ibm/private/dnssvcs_provider.go

-			case iov1.CNAMERecordType:
-				inputRData, err := p.dnsService.NewResourceRecordInputRdataRdataCnameRecord(target)
+
+			// TODO DNS record update should handle the case where we have an A record and want a CNAME record or vice versa


I think it's out of scope for this bug since it can be fixed independently.

This TODO is for when someone changes the DNSRecord type from CNAME to A, or visa versa, that it won't update the dns record type. That's not a big deal for OpenShift actually, because IBMCloud only creates CNAME records, but I can see it as a low priority "completeness" TODO.

pkg/dns/ibm/public/cis_provider.go

gcs278 · 2024-12-23T17:12:07Z

pkg/dns/ibm/public/cis_provider.go

+		}
+	}
+	if result == nil || result.Result == nil {
+		return fmt.Errorf("createOrUpdateDNSRecord: ListAllDnsRecords returned nil as result")


Hmm, that's a good question. I found that result and result.Result are both initialized when creating the DNS record for the first time. The results gets filtered for the record.Spec.DNSName, so whether it's the very first DNS record in the hosted zone isn't a factor.

I agree it's a bit odd to check for result.Result, but the original author wrote this logic, I haven't update it as works as intended so far, and it's possible there is a subtle reason why it's here.

openshift-ci · 2024-12-23T20:43:01Z

@gcs278: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/e2e-ibmcloud-operator	`2223383`	link	false	`/test e2e-ibmcloud-operator`
ci/prow/e2e-azure-ovn	`e486a51`	link	false	`/test e2e-azure-ovn`
ci/prow/e2e-aws-ovn-single-node	`e486a51`	link	false	`/test e2e-aws-ovn-single-node`

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

openshift-ci-robot added jira/severity-important Referenced Jira bug's severity is important for the branch this PR is targeting. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. labels Aug 22, 2024

openshift-ci-robot added the jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. label Aug 22, 2024

openshift-ci bot requested review from alebedev87 and frobware August 22, 2024 22:46

gcs278 force-pushed the ibm-dns-createOrUpdateDNSRecord-bug branch from b387d07 to b07d602 Compare August 22, 2024 22:50

openshift-ci-robot added the jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. label Aug 22, 2024

openshift-ci-robot removed the jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. label Aug 22, 2024

openshift-ci bot requested a review from lihongan August 22, 2024 22:57

openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 26, 2024

gcs278 force-pushed the ibm-dns-createOrUpdateDNSRecord-bug branch from a057f1a to 0716651 Compare August 26, 2024 19:28

openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 26, 2024

gcs278 force-pushed the ibm-dns-createOrUpdateDNSRecord-bug branch from 0716651 to f51b1d3 Compare August 27, 2024 14:31

Miciah reviewed Aug 28, 2024

View reviewed changes

pkg/operator/controller/ingress/load_balancer_service.go Outdated Show resolved Hide resolved

openshift-ci bot assigned Miciah Aug 28, 2024

gcs278 changed the title ~~OCPBUGS-32776: Fix IBM Public Cloud DNS Provider Update Logic~~ OCPBUGS-32776: Fix IBM Public Cloud DNS Provider Update Logic and Add Missing Instructions to the Progressing Condition Aug 28, 2024

gcs278 force-pushed the ibm-dns-createOrUpdateDNSRecord-bug branch from f51b1d3 to 7a97b9a Compare August 28, 2024 15:29

SzucsAti approved these changes Aug 28, 2024

View reviewed changes

gcs278 force-pushed the ibm-dns-createOrUpdateDNSRecord-bug branch 2 times, most recently from 32caad5 to 2223383 Compare September 16, 2024 20:55

gcs278 changed the title ~~[WIP] OCPBUGS-32776: Fix IBM Public Cloud DNS Provider Update Logic~~ OCPBUGS-32776: Fix IBM Public Cloud DNS Provider Update Logic Sep 16, 2024

openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 16, 2024

gcs278 mentioned this pull request Nov 11, 2024

[WIP] OCPBUGS-42045: Fix IBMCloud DNS Propagation Issues #1164

Draft

gcs278 force-pushed the ibm-dns-createOrUpdateDNSRecord-bug branch from 2223383 to 396acf9 Compare November 11, 2024 22:33

gcs278 force-pushed the ibm-dns-createOrUpdateDNSRecord-bug branch from 396acf9 to d93c10b Compare November 12, 2024 01:33

Miciah added the priority/backlog Higher priority than priority/awaiting-more-evidence. label Nov 13, 2024

candita reviewed Dec 10, 2024

View reviewed changes

gcs278 added 2 commits December 13, 2024 16:42

gcs278 force-pushed the ibm-dns-createOrUpdateDNSRecord-bug branch from d93c10b to e486a51 Compare December 23, 2024 17:13

gcs278 commented Dec 23, 2024

View reviewed changes

		dnsclient "github.com/openshift/cluster-ingress-operator/pkg/dns/ibm/public/client"

		"k8s.io/utils/pointer"

OCPBUGS-32776: Fix IBM Public Cloud DNS Provider Update Logic #1133

Are you sure you want to change the base?

OCPBUGS-32776: Fix IBM Public Cloud DNS Provider Update Logic #1133

Conversation

gcs278 commented Aug 22, 2024 • edited Loading

openshift-ci-robot commented Aug 22, 2024

gcs278 commented Aug 22, 2024

openshift-ci-robot commented Aug 22, 2024

openshift-ci-robot commented Aug 22, 2024

gcs278 commented Aug 26, 2024

gcs278 commented Aug 26, 2024

gcs278 commented Aug 27, 2024

gcs278 commented Aug 28, 2024

lihongan commented Aug 28, 2024

lihongan commented Aug 28, 2024

Miciah commented Aug 28, 2024

Miciah commented Aug 28, 2024

SzucsAti left a comment

Choose a reason for hiding this comment

gcs278 commented Sep 16, 2024

gcs278 commented Sep 16, 2024

gcs278 commented Sep 16, 2024

gcs278 commented Sep 17, 2024

gcs278 commented Oct 28, 2024

gcs278 commented Oct 30, 2024

gcs278 commented Oct 30, 2024

gcs278 commented Nov 11, 2024

gcs278 commented Nov 12, 2024

gcs278 commented Nov 12, 2024

openshift-ci bot commented Nov 12, 2024

gcs278 commented Nov 12, 2024

gcs278 commented Nov 12, 2024

gcs278 commented Nov 12, 2024

gcs278 commented Dec 5, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

openshift-ci bot commented Dec 23, 2024

gcs278 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

openshift-ci bot commented Dec 23, 2024

gcs278 commented Aug 22, 2024 •

edited

Loading